Add Biotech Chart Understanding Project#240
Open
sitammeur wants to merge 3 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds the Biotech Agentic Analyst: a Streamlit app that uploads PDFs, uses Datalab to convert/extract figures (with thumbnails and scores), normalizes results into typed ExtractedFigure objects, runs a CrewAI-based ScienceFlow to produce FigureIntelligence per figure with retries, and renders structured per-figure results in the UI. ChangesBiotech Agentic Analyst: Complete Application
Sequence Diagram(s)sequenceDiagram
participant User
participant Streamlit
participant Datalab
participant Flow
participant CrewAI
User->>Streamlit: upload PDF + click "Run Analysis"
Streamlit->>Datalab: run_science_pipeline(file)
Datalab->>Streamlit: convert & extract (figures + thumbnails + scores)
Streamlit->>Flow: kickoff with ExtractedFigure list
Flow->>CrewAI: kickoff per-figure (with optional image_path)
CrewAI->>Flow: FigureIntelligenceList
Flow->>Streamlit: aggregated figure_intelligences
Streamlit->>User: render per-figure intelligence UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
biotech-agentic-analyst/app.py (1)
183-247: ⚡ Quick winAI-generated text injected into
unsafe_allow_htmlHTML without escaping.Fields like
fi.key_finding,fi.biological_significance, and tag/variable names are rendered directly inside HTML strings (e.g., line 188:<strong>{fi.key_finding}</strong>). If the LLM produces output containing<,>,&,", or<script>fragments, it can break layout or, in a shared-hosting scenario, enable script injection. Escape user/AI-derived values withhtml.escape()before embedding in HTML strings.🛡️ Proposed fix (example for key_finding)
+import html as _html ... - f"{pills_html} <strong>{fi.key_finding}</strong>", + f"{pills_html} <strong>{_html.escape(fi.key_finding)}</strong>",Apply the same escaping to
fi.biological_significanceand all pill text derived from AI output.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@biotech-agentic-analyst/app.py` around lines 183 - 247, The code injects unescaped AI/user-derived strings (fi.key_finding, fi.biological_significance, fi.variables_compared, fi.knowledge_base_tags, fi.quantitative_highlights and any values passed to _pill/_axis_pill) into HTML passed to st.markdown(unsafe_allow_html=True), risking layout breakage or XSS; fix by applying html.escape() to every dynamic value before composing pills, axis labels, and the key/significance markup (i.e., escape the strings passed into _pill, _axis_pill and the string interpolated into f"{pills_html} <strong>{fi.key_finding}</strong}" and st.markdown(fi.biological_significance)), ensuring you only mark the final combined HTML as safe and do not disable escaping elsewhere.biotech-agentic-analyst/datalab_pipeline/pipeline.py (1)
113-120: 💤 Low valueSilent
except Exception: passin_poll_for_scoresswallows unexpected errors.Network timeouts and JSON parse errors are expected here, but catching
Exceptionbroadly also mutes logic bugs. Consider logging the exception at debug level so transient vs. code errors are distinguishable.♻️ Suggested improvement
try: data = _http.get(check_url, headers=headers, timeout=10).json() if "extraction_score_average" in data: return float(data["extraction_score_average"]) - except Exception: - pass + except Exception as exc: # noqa: BLE001 + print(f"[_poll_for_scores] polling error (will retry): {exc}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@biotech-agentic-analyst/datalab_pipeline/pipeline.py` around lines 113 - 120, The _poll_for_scores loop currently swallows all exceptions with a bare "except Exception: pass"; change it to catch only expected errors from the HTTP/json operations (e.g., the HTTP client request errors and JSON parsing errors raised by _http.get(...).json()) and log the exception at debug level before retrying so real logic bugs aren't muted. Specifically, replace the broad except with targeted except clauses for the request/timeout error (e.g., RequestException or your HTTP client's error type) and ValueError/JSONDecodeError for .json(), and call the module logger (or an existing logger) to log a diagnostic message that includes check_url and the caught exception while preserving the existing retry/sleep behavior for transient failures.biotech-agentic-analyst/flow/science_flow.py (1)
59-84: 💤 Low valueConsider narrowing the broad
except Exceptionin the retry loop.The catch is intentional for resilience (skip bad figures rather than abort), but swallowing truly unexpected errors (e.g.,
MemoryError,KeyboardInterrupt) makes root-cause diagnosis harder. Narrowing to expected CrewAI/LLM exceptions or at minimum re-raising non-recoverable errors would improve debuggability.♻️ Suggested narrowing
- except Exception as exc: + except (ValueError, RuntimeError, OSError) as exc:Or re-raise non-transient failures after max retries rather than just logging them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@biotech-agentic-analyst/flow/science_flow.py` around lines 59 - 84, The retry loop currently catches all Exceptions around FigureAnalystCrew().crew().kickoff(...) which can swallow non-recoverable errors; update the except block in the loop that references FigureAnalystCrew, _MAX_RETRIES, _RETRY_DELAY and last_exc so non-transient errors are re-raised (e.g., if exc is instance of KeyboardInterrupt, SystemExit, MemoryError -> raise), and narrow the caught exceptions to the expected recoverable types if you have them (e.g., CrewError, LLMError or similar) — alternatively catch Exception as exc but immediately re-raise for those non-recoverable types, keeping the existing retry/logging behavior for recoverable errors and preserving the final skip log when last_exc remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@biotech-agentic-analyst/app.py`:
- Around line 291-300: The Datalab logo is loaded from a personal GitHub repo in
the powered_by_html string used by st.markdown, which risks disappearing; copy
the image into the app's static assets (e.g., static/datalab-logo.png) or
replace the URL with an official Datalab CDN link, then update the <img> src in
the powered_by_html variable to point to the new static path or CDN URL so the
app serves a stable asset via st.markdown( powered_by_html,
unsafe_allow_html=True ).
In `@biotech-agentic-analyst/datalab_pipeline/pipeline.py`:
- Around line 229-243: The code silently treats dict items that lack both
"figures" and "figure_id" as per-page containers and can produce an empty
all_figs; after building all_figs from pages_data (using first,
page.get("figures"), etc.) detect if all_figs is empty and emit a clear warning
or error (including a short repr of first or a sample of pages_data) so
unrecognised shapes are visible; alternatively add an explicit branch when first
is a dict but lacks both keys to log/raise before falling into the per-page
case, referencing pages_data, first, and all_figs in the message.
- Around line 123-127: The parameter annotation for pipeline_id in
run_science_pipeline is declared as str but callers pass None; update the type
to accept None (e.g., Optional[str] or using PEP 604: str | None) to match usage
and preserve API compatibility, and add the corresponding import
(typing.Optional) if you choose Optional; leave the rest of run_science_pipeline
unchanged since pipeline_id is unused.
In `@biotech-agentic-analyst/flow/crews/figure_analyst/config/tasks.yaml`:
- Around line 20-22: The prompt currently forces fabrication by mandating
"minimum 2 items" for quantitative_highlights; update the task text for the
quantitative_highlights field so it does NOT require a minimum number — instruct
the model to extract up to 2 numerical results only if they are present, to
explicitly state "no quantitative data present" when none exist, and to avoid
inventing numbers; keep the biological_significance field unchanged but ensure
the instructions for quantitative_highlights and the prompt in tasks.yaml
reference the quantitative_highlights symbol and include a clear "do not
fabricate" constraint.
In `@biotech-agentic-analyst/pyproject.toml`:
- Line 6: The pyproject.toml currently sets requires-python = ">=3.14", which
conflicts with the CrewAI dependency that requires Python <3.14; update the
requires-python constraint in pyproject.toml (the requires-python key) to
include a ceiling below 3.14 (for example ">=3.10,<3.14" or ">=3.13,<3.14") so
the CrewAI requirement is satisfiable while keeping your minimum Python support.
In `@biotech-agentic-analyst/README.md`:
- Around line 15-16: Update the wording in the README where it currently reads
“a convert → extract steps” to a grammatically consistent phrase; replace it
with either “convert → extract steps” (plural) or “a convert → extract process”
(singular) so the line reads clearly, and ensure the adjacent line mentioning “A
CrewAI Flow with Analyst agent” remains unchanged.
In `@biotech-agentic-analyst/utils.py`:
- Around line 14-18: The current broad except hides errors; change the error
handling around base64.b64decode and Image.open in the function that decodes
b64_str to only catch the specific exceptions: catch binascii.Error for
base64.b64decode and catch PIL.UnidentifiedImageError, ValueError, and OSError
for Image.open, returning None for those cases and allowing other exceptions to
propagate; also add the necessary imports (binascii and UnidentifiedImageError
from PIL) so the specific exception classes can be referenced.
---
Nitpick comments:
In `@biotech-agentic-analyst/app.py`:
- Around line 183-247: The code injects unescaped AI/user-derived strings
(fi.key_finding, fi.biological_significance, fi.variables_compared,
fi.knowledge_base_tags, fi.quantitative_highlights and any values passed to
_pill/_axis_pill) into HTML passed to st.markdown(unsafe_allow_html=True),
risking layout breakage or XSS; fix by applying html.escape() to every dynamic
value before composing pills, axis labels, and the key/significance markup
(i.e., escape the strings passed into _pill, _axis_pill and the string
interpolated into f"{pills_html} <strong>{fi.key_finding}</strong}"
and st.markdown(fi.biological_significance)), ensuring you only mark the final
combined HTML as safe and do not disable escaping elsewhere.
In `@biotech-agentic-analyst/datalab_pipeline/pipeline.py`:
- Around line 113-120: The _poll_for_scores loop currently swallows all
exceptions with a bare "except Exception: pass"; change it to catch only
expected errors from the HTTP/json operations (e.g., the HTTP client request
errors and JSON parsing errors raised by _http.get(...).json()) and log the
exception at debug level before retrying so real logic bugs aren't muted.
Specifically, replace the broad except with targeted except clauses for the
request/timeout error (e.g., RequestException or your HTTP client's error type)
and ValueError/JSONDecodeError for .json(), and call the module logger (or an
existing logger) to log a diagnostic message that includes check_url and the
caught exception while preserving the existing retry/sleep behavior for
transient failures.
In `@biotech-agentic-analyst/flow/science_flow.py`:
- Around line 59-84: The retry loop currently catches all Exceptions around
FigureAnalystCrew().crew().kickoff(...) which can swallow non-recoverable
errors; update the except block in the loop that references FigureAnalystCrew,
_MAX_RETRIES, _RETRY_DELAY and last_exc so non-transient errors are re-raised
(e.g., if exc is instance of KeyboardInterrupt, SystemExit, MemoryError ->
raise), and narrow the caught exceptions to the expected recoverable types if
you have them (e.g., CrewError, LLMError or similar) — alternatively catch
Exception as exc but immediately re-raise for those non-recoverable types,
keeping the existing retry/logging behavior for recoverable errors and
preserving the final skip log when last_exc remains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b19f05e3-57bb-481c-a27b-7058eff35a05
⛔ Files ignored due to path filters (1)
biotech-agentic-analyst/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
biotech-agentic-analyst/.env.examplebiotech-agentic-analyst/.streamlit/config.tomlbiotech-agentic-analyst/README.mdbiotech-agentic-analyst/app.pybiotech-agentic-analyst/datalab_pipeline/__init__.pybiotech-agentic-analyst/datalab_pipeline/pipeline.pybiotech-agentic-analyst/flow/__init__.pybiotech-agentic-analyst/flow/crews/__init__.pybiotech-agentic-analyst/flow/crews/figure_analyst/__init__.pybiotech-agentic-analyst/flow/crews/figure_analyst/config/agents.yamlbiotech-agentic-analyst/flow/crews/figure_analyst/config/tasks.yamlbiotech-agentic-analyst/flow/crews/figure_analyst/crew.pybiotech-agentic-analyst/flow/science_flow.pybiotech-agentic-analyst/flow/state.pybiotech-agentic-analyst/models.pybiotech-agentic-analyst/pyproject.tomlbiotech-agentic-analyst/utils.py
Comment on lines
+123
to
+127
| def run_science_pipeline( | ||
| client, | ||
| pipeline_id: str, # kept for API compatibility, not used | ||
| file_path: str, | ||
| poll_interval: int = 1, |
Contributor
There was a problem hiding this comment.
pipeline_id type annotation is str but callers pass None.
app.py calls run_science_pipeline(client, None, tmp_path), which violates the pipeline_id: str annotation. Since the parameter is intentionally unused, annotate it as Optional[str] (or str | None) to match actual usage.
🛠️ Proposed fix
def run_science_pipeline(
client,
- pipeline_id: str, # kept for API compatibility, not used
+ pipeline_id: str | None = None, # kept for API compatibility, not used
file_path: str,
poll_interval: int = 1,
timeout: int = 1800,
) -> dict:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def run_science_pipeline( | |
| client, | |
| pipeline_id: str, # kept for API compatibility, not used | |
| file_path: str, | |
| poll_interval: int = 1, | |
| def run_science_pipeline( | |
| client, | |
| pipeline_id: str | None = None, # kept for API compatibility, not used | |
| file_path: str, | |
| poll_interval: int = 1, | |
| timeout: int = 1800, | |
| ) -> dict: |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@biotech-agentic-analyst/datalab_pipeline/pipeline.py` around lines 123 - 127,
The parameter annotation for pipeline_id in run_science_pipeline is declared as
str but callers pass None; update the type to accept None (e.g., Optional[str]
or using PEP 604: str | None) to match usage and preserve API compatibility, and
add the corresponding import (typing.Optional) if you choose Optional; leave the
rest of run_science_pipeline unchanged since pipeline_id is unused.
Comment on lines
+229
to
+243
| elif isinstance(pages_data, list): | ||
| # Detect flat list of figure objects vs list of per-page dicts. | ||
| # A per-page dict has a "figures" key; a figure object has "figure_id". | ||
| first = next((item for item in pages_data if isinstance(item, dict)), None) | ||
| if first is not None and "figures" not in first and "figure_id" in first: | ||
| # flat list of figure objects | ||
| all_figs = [item for item in pages_data if isinstance(item, dict)] | ||
| else: | ||
| # list of per-page dicts: [{"figures": [...]}, ...] | ||
| all_figs = [ | ||
| f | ||
| for page in pages_data | ||
| if isinstance(page, dict) | ||
| for f in (page.get("figures") or []) | ||
| ] |
Contributor
There was a problem hiding this comment.
Silent fallthrough for list items with neither "figures" nor "figure_id" key.
At line 233, if first is a dict with neither key (e.g. an unexpected wrapper shape), the code falls through to the else branch and treats each item as a per-page container, calling page.get("figures") on it. This returns None for every item, silently producing zero figures with no diagnostic. At minimum, log the unrecognised shape when all_figs ends up empty.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@biotech-agentic-analyst/datalab_pipeline/pipeline.py` around lines 229 - 243,
The code silently treats dict items that lack both "figures" and "figure_id" as
per-page containers and can produce an empty all_figs; after building all_figs
from pages_data (using first, page.get("figures"), etc.) detect if all_figs is
empty and emit a clear warning or error (including a short repr of first or a
sample of pages_data) so unrecognised shapes are visible; alternatively add an
explicit branch when first is a dict but lacks both keys to log/raise before
falling into the per-page case, referencing pages_data, first, and all_figs in
the message.
Comment on lines
+20
to
+22
| - quantitative_highlights: list every numerical result, p-value, fold-change, or percentage | ||
| difference readable from the figure or caption — minimum 2 items per figure | ||
| - biological_significance: 2-3 sentences on why this finding matters scientifically or |
Contributor
There was a problem hiding this comment.
Current prompt can force fabricated quantitative outputs.
Requiring a minimum of 2 quantitative highlights per figure (Line 21) will push the model to invent numbers when a figure has sparse/no numeric content.
Proposed prompt adjustment
- - quantitative_highlights: list every numerical result, p-value, fold-change, or percentage
- difference readable from the figure or caption — minimum 2 items per figure
+ - quantitative_highlights: list all numerical results, p-values, fold-changes, or percentages
+ explicitly readable from the figure or caption; if none are present, return an empty list🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@biotech-agentic-analyst/flow/crews/figure_analyst/config/tasks.yaml` around
lines 20 - 22, The prompt currently forces fabrication by mandating "minimum 2
items" for quantitative_highlights; update the task text for the
quantitative_highlights field so it does NOT require a minimum number — instruct
the model to extract up to 2 numerical results only if they are present, to
explicitly state "no quantitative data present" when none exist, and to avoid
inventing numbers; keep the biological_significance field unchanged but ensure
the instructions for quantitative_highlights and the prompt in tasks.yaml
reference the quantitative_highlights symbol and include a clear "do not
fabricate" constraint.
| version = "0.1.0" | ||
| description = "Agentic scientific figure intelligence workflow for the biotech domain" | ||
| readme = "README.md" | ||
| requires-python = ">=3.14" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is Python 3.14 broadly available across common CI and hosting platforms (GitHub Actions, Streamlit Cloud, Docker base images), and what is the recommended minimum Python version for CrewAI + Streamlit projects in 2026?
💡 Result:
No, Python 3.14 is not broadly available across all specified platforms due to CrewAI's upper version limit. GitHub Actions supports Python 3.14 via actions/setup-python and actions/python-versions releases up to 3.14.4, with "3.x" resolving to 3.14.x stable[1][2][3]. Streamlit library supports Python 3.10-3.14[4], and CI runs on 3.14[5][6]; Streamlit Cloud supports released Python versions receiving security updates (3.14 qualifies, though exact list unspecified)[7]. Official Docker Python images include tags like python:3.14, 3.14.4-alpine, etc., across multiple architectures and bases[8][9]. However, CrewAI officially requires Python >=3.10 and <3.14 (or <=3.13 per PyPI), so it does not support 3.14[10][11]. The recommended minimum Python version for CrewAI + Streamlit projects in 2026 is 3.10, as per CrewAI's requirement (>=3.10)[10], which aligns with Streamlit's support starting at 3.10[4]. Use 3.13 for latest compatible with CrewAI while nearing 3.14 support.
Citations:
- 1: https://github.com/actions/python-versions/releases
- 2: https://github.com/actions/setup-python/
- 3: setup-python@v6 with python-version: "3.x" installs pre-release Python 3.14.2 instead of latest stable version actions/setup-python#1275
- 4: https://docs.streamlit.io/knowledge-base/using-streamlit/sanity-checks
- 5: Add support for Python 3.14 streamlit/streamlit#12967
- 6: https://github.com/streamlit/streamlit/actions/runs/23029832018
- 7: https://docs.streamlit.io/deploy/streamlit-community-cloud/status
- 8: https://github.com/docker-library/official-images/blob/master/library/python
- 9: https://store.docker.com/images/python/
- 10: https://docs.crewai.com/installation
- 11: https://pypi.org/project/crewai/0.74.1/
🏁 Script executed:
grep -n -A 5 -B 5 "crewai\|CrewAI" biotech-agentic-analyst/pyproject.tomlRepository: patchy631/ai-engineering-hub
Length of output: 423
Remove Python 3.14 floor — CrewAI dependency is incompatible.
Line 6 sets requires-python = ">=3.14", but CrewAI (line 8 dependency) officially requires <3.14 per PyPI. This creates an unsolvable constraint that will fail all installations. Change to requires-python = ">=3.10,<3.14" or >=3.13 to align with CrewAI's maximum supported version.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@biotech-agentic-analyst/pyproject.toml` at line 6, The pyproject.toml
currently sets requires-python = ">=3.14", which conflicts with the CrewAI
dependency that requires Python <3.14; update the requires-python constraint in
pyproject.toml (the requires-python key) to include a ceiling below 3.14 (for
example ">=3.10,<3.14" or ">=3.13,<3.14") so the CrewAI requirement is
satisfiable while keeping your minimum Python support.
Comment on lines
+15
to
+16
| 2. Datalab runs a convert → extract steps to identify and extract all figures, axis labels, and captions | ||
| 3. A CrewAI Flow with Analyst agent produces structured intelligence for each figure: |
Contributor
There was a problem hiding this comment.
Fix wording in workflow step for clarity.
Line 15 currently says “a convert → extract steps”; this should be singular/plural consistent (e.g., “convert → extract steps” or “a convert → extract process”).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@biotech-agentic-analyst/README.md` around lines 15 - 16, Update the wording
in the README where it currently reads “a convert → extract steps” to a
grammatically consistent phrase; replace it with either “convert → extract
steps” (plural) or “a convert → extract process” (singular) so the line reads
clearly, and ensure the adjacent line mentioning “A CrewAI Flow with Analyst
agent” remains unchanged.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
biotech-agentic-analyst/datalab_pipeline/pipeline.py (1)
229-243:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit handling for unknown list item shapes to avoid silent empty results.
If list entries are dicts without both
figuresandfigure_id, this currently falls through and can silently produce zero extracted figures.💡 Suggested fix
elif isinstance(pages_data, list): # Detect flat list of figure objects vs list of per-page dicts. # A per-page dict has a "figures" key; a figure object has "figure_id". first = next((item for item in pages_data if isinstance(item, dict)), None) if first is not None and "figures" not in first and "figure_id" in first: # flat list of figure objects all_figs = [item for item in pages_data if isinstance(item, dict)] + elif first is not None and "figures" not in first and "figure_id" not in first: + raise ValueError( + "Unrecognised list item shape in extraction_schema_json: " + f"sample={str(first)[:200]}" + ) else: # list of per-page dicts: [{"figures": [...]}, ...] all_figs = [🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@biotech-agentic-analyst/datalab_pipeline/pipeline.py` around lines 229 - 243, The code currently treats a list of dicts without "figures" or "figure_id" as a per-page list and can silently yield no figures; update the pages_data handling (in the branch that computes all_figs using first, pages_data, and page.get("figures")) to explicitly detect when a dict item lacks both "figures" and "figure_id" and fail fast: either raise a ValueError (including a short message with the offending item's keys or a sample item) or emit a clear warning and skip those items, ensuring the function does not silently return an empty all_figs when input shapes are unknown.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@biotech-agentic-analyst/datalab_pipeline/pipeline.py`:
- Line 193: The current assignment for quality_score uses a truthiness check
which turns valid zero values into None; update the logic that sets
quality_score (using the _raw_quality variable in pipeline.py) to explicitly
detect missing values (e.g., None or empty string) instead of relying on
truthiness, then parse with float(_raw_quality) when present—this preserves 0
and 0.0 while still mapping truly absent inputs to None and should include a
safe parse (try/except) if parsing failures must be handled.
- Around line 123-129: The function signature for run_science_pipeline is
invalid because a parameter with a default (pipeline_id: str | None = None)
appears before a parameter without a default (file_path: str); reorder
parameters so all required (non-default) parameters come first — for example put
file_path: str before pipeline_id: str | None = None — or alternatively give
file_path a default if intended; update the run_science_pipeline definition
accordingly and keep the same parameter names and types to preserve callers and
API compatibility.
---
Duplicate comments:
In `@biotech-agentic-analyst/datalab_pipeline/pipeline.py`:
- Around line 229-243: The code currently treats a list of dicts without
"figures" or "figure_id" as a per-page list and can silently yield no figures;
update the pages_data handling (in the branch that computes all_figs using
first, pages_data, and page.get("figures")) to explicitly detect when a dict
item lacks both "figures" and "figure_id" and fail fast: either raise a
ValueError (including a short message with the offending item's keys or a sample
item) or emit a clear warning and skip those items, ensuring the function does
not silently return an empty all_figs when input shapes are unknown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9b42ca8f-7d00-46c1-9759-34e46bd3c6c7
📒 Files selected for processing (4)
biotech-agentic-analyst/README.mdbiotech-agentic-analyst/datalab_pipeline/pipeline.pybiotech-agentic-analyst/flow/crews/figure_analyst/config/tasks.yamlbiotech-agentic-analyst/pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- biotech-agentic-analyst/pyproject.toml
- biotech-agentic-analyst/flow/crews/figure_analyst/config/tasks.yaml
Comment on lines
+123
to
+129
| def run_science_pipeline( | ||
| client, | ||
| pipeline_id: str | None = None, # kept for API compatibility, not used | ||
| file_path: str, | ||
| poll_interval: int = 1, | ||
| timeout: int = 1800, | ||
| ) -> dict: |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the invalid parameter-order pattern is present/removed.
rg -nP 'def run_science_pipeline\(\n\s*client,\n\s*pipeline_id:.*=\s*None,\n\s*file_path:' biotech-agentic-analyst/datalab_pipeline/pipeline.pyRepository: patchy631/ai-engineering-hub
Length of output: 54
🏁 Script executed:
sed -n '120,135p' biotech-agentic-analyst/datalab_pipeline/pipeline.pyRepository: patchy631/ai-engineering-hub
Length of output: 521
Fix function signature — current parameter ordering is invalid Python syntax.
pipeline_id has a default value but file_path does not. In Python, parameters with defaults must come after parameters without defaults. This causes a syntax error and prevents importing this module.
Suggested fix
def run_science_pipeline(
client,
- pipeline_id: str | None = None, # kept for API compatibility, not used
+ pipeline_id: str | None, # kept for API compatibility, not used
file_path: str,
poll_interval: int = 1,
timeout: int = 1800,
) -> dict:🧰 Tools
🪛 Ruff (0.15.12)
[warning] 126-126: Parameter without a default cannot follow a parameter with a default
(invalid-syntax)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@biotech-agentic-analyst/datalab_pipeline/pipeline.py` around lines 123 - 129,
The function signature for run_science_pipeline is invalid because a parameter
with a default (pipeline_id: str | None = None) appears before a parameter
without a default (file_path: str); reorder parameters so all required
(non-default) parameters come first — for example put file_path: str before
pipeline_id: str | None = None — or alternatively give file_path a default if
intended; update the run_science_pipeline definition accordingly and keep the
same parameter names and types to preserve callers and API compatibility.
|
|
||
| page_count: int = getattr(convert_result, "page_count", 0) or 0 | ||
| _raw_quality = getattr(convert_result, "parse_quality_score", None) | ||
| quality_score: float | None = float(_raw_quality) if _raw_quality else None |
Contributor
There was a problem hiding this comment.
Preserve valid zero parse-quality values.
Using a truthiness check here converts 0/0.0 to None, which loses a valid score.
💡 Suggested fix
- quality_score: float | None = float(_raw_quality) if _raw_quality else None
+ quality_score: float | None = (
+ float(_raw_quality) if _raw_quality is not None else None
+ )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@biotech-agentic-analyst/datalab_pipeline/pipeline.py` at line 193, The
current assignment for quality_score uses a truthiness check which turns valid
zero values into None; update the logic that sets quality_score (using the
_raw_quality variable in pipeline.py) to explicitly detect missing values (e.g.,
None or empty string) instead of relying on truthiness, then parse with
float(_raw_quality) when present—this preserves 0 and 0.0 while still mapping
truly absent inputs to None and should include a safe parse (try/except) if
parsing failures must be handled.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@biotech-agentic-analyst/datalab_pipeline/pipeline.py`:
- Around line 117-119: The try/except around the polling request in pipeline.py
is silently swallowing all errors; change it to catch specific failures (e.g.,
requests.exceptions.RequestException, json.JSONDecodeError, KeyError) instead of
a bare Exception and log the exception (use the project logger like
process_logger or logger) at debug or error level with context (URL, attempt
count) before continuing; keep the existing sleep(2) loop and ensure the logger
import is added/adjusted to match the rest of the project.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7b52ce43-6c58-4254-b611-0a22604a4fda
📒 Files selected for processing (1)
biotech-agentic-analyst/datalab_pipeline/pipeline.py
Comment on lines
+117
to
+119
| except Exception: | ||
| pass | ||
| time.sleep(2) |
Contributor
There was a problem hiding this comment.
Don't silently swallow polling errors.
The bare except Exception: pass hides every failure mode here — auth errors, malformed JSON, transport failures, etc. — so you'll spin until max_wait with no diagnostic when the check URL is misconfigured or returning HTML/text. At minimum, log a debug-level message; ideally narrow the exception types.
🛡️ Proposed fix
- while time.time() < deadline:
- try:
- data = _http.get(check_url, headers=headers, timeout=10).json()
- if "extraction_score_average" in data:
- return float(data["extraction_score_average"])
- except Exception:
- pass
- time.sleep(2)
+ while time.time() < deadline:
+ try:
+ resp = _http.get(check_url, headers=headers, timeout=10)
+ resp.raise_for_status()
+ data = resp.json()
+ if "extraction_score_average" in data:
+ return float(data["extraction_score_average"])
+ except (_http.RequestException, ValueError) as exc:
+ logger.debug("score poll failed: %s", exc)
+ time.sleep(2)(adjust the logger import to match the rest of the project)
🧰 Tools
🪛 Ruff (0.15.12)
[error] 117-118: try-except-pass detected, consider logging the exception
(S110)
[warning] 117-117: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@biotech-agentic-analyst/datalab_pipeline/pipeline.py` around lines 117 - 119,
The try/except around the polling request in pipeline.py is silently swallowing
all errors; change it to catch specific failures (e.g.,
requests.exceptions.RequestException, json.JSONDecodeError, KeyError) instead of
a bare Exception and log the exception (use the project logger like
process_logger or logger) at debug or error level with context (URL, attempt
count) before continuing; keep the existing sleep(2) loop and ensure the logger
import is added/adjusted to match the rest of the project.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores